Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve unknown keysym handling by reusing keycodes #1734

Merged
merged 1 commit into from
Aug 6, 2024
Merged

Improve unknown keysym handling by reusing keycodes #1734

merged 1 commit into from
Aug 6, 2024

Conversation

gujjwal00
Copy link
Contributor

@gujjwal00 gujjwal00 commented Mar 4, 2024 •
edited
Loading

This is still a work in progress, but I am kinda stuck on how to handle Xvnc. Atleast x0vncserver works correctly. As proposed in #93 (comment), when all free keys has been used, it will start reusing the oldest key assigned by TigerVNC.

For Xvnc, I have tried a few things:

  • Tried to compile common source files as part of Xvnc. But internal xserver header files don't seem to be compatible with C++. This also complicates the build process even more as the common header file needs to know if it is being compiled with Xvnc or x0vncserver.
  • Tried to separate server specific parts to something like InputGlue.h. This can work, but becomes very clunky. Control flow will be Xvnc/x0vncserver => common code => glue code in Xvnc/x0vncserver => and back again.
  • We could of course implement this separately in Xvnc & x0vncserver, without worrying too much about duplication. (I would still like to keep the custom key list in common to avoid having to implement a linked list in Xvnc.)

Issue: #93

@CendioOssman CendioOssman marked this pull request as draft March 26, 2024 09:06
@gujjwal00 gujjwal00 force-pushed the fix-unknown-keysym branch 2 times, most recently from 6c69583 to 49bc636 Compare April 8, 2024 12:27
@gujjwal00 gujjwal00 changed the title Handle unknown keysyms by using unsed keys Improve unknown keysym handling by reusing keycodes Apr 8, 2024
@gujjwal00
Copy link
Contributor Author

I have added separate implementations for x0vnceserver & xvnc.

Trying to share this piece of code creates too many problems because both servers are using different headers, different way of getting/updating keymaps, and different build systems.

@gujjwal00 gujjwal00 marked this pull request as ready for review April 8, 2024 12:53
CendioOssman
CendioOssman approved these changes Jun 24, 2024
View reviewed changes
Copy link
Member

@CendioOssman CendioOssman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the huge delay reviewing this.

I think this looks reasonable, but I haven't done any actual testing yet.

Clever using the naming to track our modified keys. :)
Have you checked that the naming resets when you modiify the keyboard layout using other tools?

@CendioOssman CendioOssman requested a review from samhed June 24, 2024 10:10
@gujjwal00
Improve unknown keysym handling
f65433a 
Instead of giving up after all free keycodes have been used, Keycodes from previously added keysyms will be reused.

Re: TigerVNC#93
@gujjwal00
Copy link
Contributor Author

Re-based onto latest master branch.

Have you checked that the naming resets when you modiify the keyboard layout using other tools?

Yes, changing layout via setxkbmap will reset the naming. Basically, any operation which triggers reload of XKB configuration from disk will reset in-memory changes done by Tigervnc. Please note that in case of multi-layout configuration, fast layout switching via key shortcut doesn't reload the configuration from disk, it only changes the active group. But its not an issue here because Tigervnc only modifies unused keys.

@gujjwal00
Copy link
Contributor Author

Also, only Xvnc assigns key name to added keys. x0vncserver simply updates the keysym. But if needed, same approach could be used in x0vncserver too.

@CendioOssman
Copy link
Member

Finally got around to testing this. It works very well. Nice work! :)

Also, only Xvnc assigns key name to added keys. x0vncserver simply updates the keysym. But if needed, same approach could be used in x0vncserver too.

That would be nice if you have the time. It's not a blocker for this work, though, so I'll go ahead and merge what we have.

@CendioOssman CendioOssman merged commit e2bbd9f into TigerVNC:master Aug 6, 2024
@CendioOssman CendioOssman mentioned this pull request Aug 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CendioOssman CendioOssman CendioOssman approved these changes

@samhed samhed Awaiting requested review from samhed

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gujjwal00 @CendioOssman